Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(actix): capture HTTP request body #731

Merged
merged 18 commits into from
Feb 14, 2025

Conversation

pacifistes
Copy link
Contributor

@pacifistes pacifistes commented Feb 2, 2025

Summary

This PR adds the ability to capture HTTP request bodies in the Sentry Actix middleware, allowing for better debugging and error tracking capabilities. The feature is configurable through the max_request_body_size option in ClientOptions.

Changes

  • Added MaxRequestBodySize enum to control request body capture behavior:
    • None: Don't capture request bodies (default)
    • Small: Capture up to 1000 bytes
    • Medium: Capture up to 10000 bytes
    • Always: Capture entire body
  • Added request body capture support for JSON and form-urlencoded content types
  • Implemented body capture and restoration logic to maintain request integrity
  • Added necessary dependencies: serde_json, actix-http

Implementation Details

  • Request bodies are only captured for content types application/json and application/x-www-form-urlencoded
  • The body is read from the request stream and then restored to maintain middleware transparency
  • Size limits are enforced based on the configured max_request_body_size option
  • Empty string is returned for bodies exceeding size limits

Add MaxRequestBodySize enum to control request body capture with options:
- None: Don't capture request body (default)
- Small: Capture up to 1000 bytes
- Medium: Capture up to 10000 bytes
- Always: Capture entire body

Add max_request_body_size field to ClientOptions struct with default value of None
Add support for capturing request bodies in the Sentry middleware for Actix-Web.
This includes:
- Configurable request body size limits (Small/Medium/Always)
- Support for JSON and form-urlencoded content types
- Body capture and restoration logic to maintain request integrity
- Additional span data enrichment

Updates dependencies:
- Add serde_json, actix-http, futures dependencies
@lcian lcian self-assigned this Feb 2, 2025
@Swatinem
Copy link
Member

Swatinem commented Feb 4, 2025

Another question would be: Is the intention to capture up to the configured limit, or to discard any payloads that exceed the limit.

@lcian
Copy link
Member

lcian commented Feb 4, 2025

Another question would be: Is the intention to capture up to the configured limit, or to discard any payloads that exceed the limit.

The goal here is to discard payloads that exceed the limit. In some SDKs (e.g. Python) there is some more complex logic to handle payloads that exceed the max but in others (e.g. PHP) it's simply discarded.
So for now I think we can go for the simpler option.

pacifistes and others added 6 commits February 4, 2025 21:48
Implements a method to validate request body sizes against predefined limits:
- None: Don't capture request bodies (default)
- Small: Capture up to 1000 bytes
- Medium: Capture up to 10000 bytes
- Always: Capture entire body
- Add chunked transfer encoding check to prevent capturing chunked requests
- Add strict content-type validation for JSON and form-urlencoded
- Implement content length validation against size limits
Add new `Explicit(usize)` variant to `MaxRequestBodySize` enum, allowing users
to specify custom maximum request body size limits for event capture.
@pacifistes pacifistes requested review from lcian and Swatinem February 4, 2025 22:24
@lcian
Copy link
Member

lcian commented Feb 14, 2025

Setting the option to Medium by default as that's what all of the other SDKs do, the develop docs were outdated.

@lcian lcian changed the title Feat: Get request body in the issue using actix_web and sentry-actix feat(actix): capture HTTP request body Feb 14, 2025
@lcian
Copy link
Member

lcian commented Feb 14, 2025

@pacifistes thanks for the contribution! 😄

@lcian lcian merged commit 46022c4 into getsentry:master Feb 14, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants